Conversation
There was a problem hiding this comment.
Pull request overview
This pull request modifies the etaDihadron.cxx file to change histogram tracking from transverse momentum (pT) to pseudorapidity (eta) for trigger and associated particles in di-hadron correlation analysis. The changes affect histogram definitions, axis specifications, and data filling calls.
Changes:
- Updated histogram axes and fill calls from pT to eta for trigger histograms (Trig_hist, MCTrig_hist)
- Changed histogram names and titles from "pt" to "eta" for solo track histograms (Nch_final_eta, Solo_tracks_trigger, Solo_tracks_assoc)
- Minor formatting fix removing space in cast expression
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (std::find(tracksSkipIndices.begin(), tracksSkipIndices.end(), track1.globalIndex()) != tracksSkipIndices.end()) { | ||
| registry.fill(HIST("Solo_tracks_trigger"), track1.pt()); | ||
| registry.fill(HIST("Solo_tracks_trigger"), track1.eta()); | ||
| continue; // Skip the track1 if it is alone in pt bin |
There was a problem hiding this comment.
The comment says "Skip the track1 if it is alone in pt bin" but the code has been changed to fill histograms with eta values instead of pt values. However, the underlying logic in findBiasedTracks (line 838-875) still identifies tracks that are alone in their pT bins, not eta bins. This creates a semantic inconsistency - the function identifies solo tracks based on pT binning, but the histogram is now filled with eta values. Either the comment should be updated to clarify that tracks are identified as solo based on pT bins but their eta is recorded, or the entire feature should be reconsidered for consistency.
| if (std::find(tracks2SkipIndices.begin(), tracks2SkipIndices.end(), track2.globalIndex()) != tracks2SkipIndices.end()) { | ||
| registry.fill(HIST("Solo_tracks_assoc"), track2.pt()); | ||
| registry.fill(HIST("Solo_tracks_assoc"), track2.eta()); | ||
| continue; // Skip the track2 if it is alone in pt bin |
There was a problem hiding this comment.
The comment says "Skip the track2 if it is alone in pt bin" but the code has been changed to fill histograms with eta values instead of pt values. However, the underlying logic in findBiasedTracks (line 838-875) still identifies tracks that are alone in their pT bins, not eta bins. This creates a semantic inconsistency - the function identifies solo tracks based on pT binning, but the histogram is now filled with eta values. Either the comment should be updated to clarify that tracks are identified as solo based on pT bins but their eta is recorded, or the entire feature should be reconsidered for consistency.
| registry.add("Nch_final_eta", "eta", {HistType::kTH1D, {axisEtaTrigger}}); | ||
| registry.add("Solo_tracks_trigger", "eta", {HistType::kTH1D, {axisEtaTrigger}}); | ||
| if (!cfgSingleSoloPtTrack) { | ||
| registry.add("Solo_tracks_assoc", "pT", {HistType::kTH1D, {axisEtaAssoc}}); | ||
| registry.add("Solo_tracks_assoc", "eta", {HistType::kTH1D, {axisEtaAssoc}}); |
There was a problem hiding this comment.
The histogram names have been changed from "Nch_final_pt", "Solo_tracks_trigger", and "Solo_tracks_assoc" to use "eta" instead of "pt", and they are now filled with eta values. However, the configuration variable cfgSoloPtTrack (line 103) and the function findBiasedTracks (line 838-875) still refer to pT bins. This creates semantic confusion - the feature is configured and documented as being about pT bins, but the histograms now record eta values. Consider either: (1) keeping the original pT-based histogram fills to maintain consistency with the feature's purpose, or (2) if eta tracking is intentional, update the configuration variable names and function names to reflect this change in behavior.
| @@ -282,10 +282,10 @@ struct EtaDihadron { | |||
| registry.add("Trig_hist", "", {HistType::kTHnSparseF, {{axisSample, axisVertex, axisEtaTrigger}}}); | |||
There was a problem hiding this comment.
The histogram definition now uses axisEtaTrigger and tracks eta values, which is inconsistent with a similar implementation in PWGCF/TwoParticleCorrelations/Tasks/diHadronCor.cxx (lines 282, 285-288) where Trig_hist and solo track histograms use axisPtTrigger and track pT values. If this divergence is intentional for eta-specific analysis, consider documenting the rationale. Otherwise, these implementations should remain consistent across the codebase.
1 participant
No description provided.